-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(nestjs): Add support for Symbol as event name #17785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(nestjs): Add support for Symbol as event name #17785
Conversation
The @onevent decorator accepts the following types for its argument: ```typescript string | symbol | Array<string | symbol> `` If a Symbol is included in an array, the code to get the eventName will throw a TypeError (String(event)) This occurs because JavaScript’s Array.prototype.join internally calls ToString on each array element. Per the specification, ToString(Symbol) is not allowed and results in a TypeError. To avoid this issue, do not rely on String(array) or .join() on arrays containing symbols directly. Instead, explicitly convert each element to a string while handling symbols safely. doc: https://tc39.es/ecma262/multipage/indexed-collections.html#sec-array.prototype.join doc: https://tc39.es/ecma262/multipage/abstract-operations.html#sec-tostring
} else if (Array.isArray(event)) { | ||
return event.map(eventNameFromEvent).join(','); | ||
} else return String(event); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should work: https://tc39.es/ecma262/multipage/text-processing.html#sec-string-constructor-string-value, but let me know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me, thanks for contributing this!
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #17785 Co-authored-by: chargome <[email protected]>
The `@OnEvent` decorator accepts the following types for its argument: ```typescript string | symbol | Array<string | symbol> ``` If a Symbol is included in an array, the code to get the eventName will throw a TypeError (String(event)) This occurs because JavaScript’s Array.prototype.join internally calls ToString on each array element. Per the specification, ToString(Symbol) is not allowed and results in a TypeError. To avoid this issue, do not rely on String(array) or .join() on arrays containing symbols directly. Instead, explicitly convert each element to a string while handling symbols safely. I couldn't find a way to test adding multiple `@OnEvent` so the second part can be tested. It didn't have any tests before (as far as I can tell), but would be nice to add them. doc: https://tc39.es/ecma262/multipage/indexed-collections.html#sec-array.prototype.join doc: https://tc39.es/ecma262/multipage/abstract-operations.html#sec-tostring
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See getsentry#17785 Co-authored-by: chargome <[email protected]>
The
@OnEvent
decorator accepts the following types for its argument:If a Symbol is included in an array, the code to get the eventName will throw a TypeError (String(event)) This occurs because JavaScript’s
Array.prototype.join internally calls ToString on each array element. Per the specification, ToString(Symbol) is not allowed and results in a TypeError.
To avoid this issue, do not rely on String(array) or .join() on arrays containing symbols directly. Instead, explicitly convert each element to a string while handling symbols safely.
I couldn't find a way to test adding multiple
@OnEvent
so the second part can be tested. It didn't have any tests before (as far as I can tell), but would be nice to add them.doc: https://tc39.es/ecma262/multipage/indexed-collections.html#sec-array.prototype.join
doc: https://tc39.es/ecma262/multipage/abstract-operations.html#sec-tostring